Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(protocol-designer): add custom liquid color picker #10958

Merged
merged 16 commits into from
Jul 11, 2022
Merged

feat(protocol-designer): add custom liquid color picker #10958

merged 16 commits into from
Jul 11, 2022

Conversation

sakibh
Copy link
Contributor

@sakibh sakibh commented Jun 30, 2022 •
edited
Loading

Overview

This PR adds a custom liquid color picker and field to the liquids page in Protocol Designer. closes #10573 , closes #10601

Co-authored by @smb2268

image

Note: still working on updating/adding tests

Changelog

  • Added react-color
  • Added displayColor field to liquids page component and redux store
  • Updated PD liquids colors

Review requests

Note: In order to access this new feature, enable the feature flag called Enable liquid color enhancements.

  • Check that the ACs for both tickets mentioned above are fulfilled.
  • Check that you are able to choose any of the predefined colors AND/OR add a custom color via hex code.
  • Check that the colors are persisting once you click save.
  • Check that when adding liquid to labware, the colors are accurate/persistent.
  • Check that when uploading an old protocol, PD prompts you to update the version and the liquid colors should default to new the new color scheme.

Risk assessment

low, behind FF

@sakibh sakibh requested review from b-cooper, shlokamin and smb2268 June 30, 2022 15:13
@sakibh sakibh requested review from a team as code owners June 30, 2022 15:13
@sakibh sakibh self-assigned this Jun 30, 2022
@sakibh sakibh removed request for a team June 30, 2022 15:13
@codecov
Copy link

codecov bot commented Jun 30, 2022 •
edited
Loading

Codecov Report

Merging #10958 (0f39146) into edge (ff3dc71) will increase coverage by 0.27%.
The diff coverage is 36.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             edge   #10958      +/-   ##
==========================================
+ Coverage   73.78%   74.06%   +0.27%     
==========================================
  Files        2076     2034      -42     
  Lines       57332    56231    -1101     
  Branches     5731     5550     -181     
==========================================
- Hits        42304    41645     -659     
+ Misses      13790    13377     -413     
+ Partials     1238     1209      -29
Flag Coverage Δ
app 72.10% <ø> (+1.21%) ⬆️
components 53.10% <100.00%> (+0.32%) ⬆️
labware-library 49.74% <ø> (ø)
protocol-designer 45.88% <17.94%> (-0.15%) ⬇️
react-api-client 83.05% <ø> (ø)
step-generation 89.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ocol-designer/src/components/ColorPicker/index.tsx 0.00% <0.00%> (ø)
...esigner/src/components/DeckSetup/LabwareOnDeck.tsx 30.00% <ø> (ø)
...-designer/src/components/IngredientsList/index.tsx 0.00% <0.00%> (ø)
...l-designer/src/components/LiquidPlacementModal.tsx 0.00% <0.00%> (ø)
...gner/src/components/LiquidsPage/LiquidEditForm.tsx 0.00% <0.00%> (ø)
...ocol-designer/src/components/LiquidsPage/index.tsx 0.00% <ø> (ø)
...l-designer/src/components/LiquidsSidebar/index.tsx 0.00% <0.00%> (ø)
...m/fields/WellSelectionField/WellSelectionModal.tsx 6.25% <0.00%> (-0.21%) ⬇️
...signer/src/components/labware/BrowsableLabware.tsx 0.00% <0.00%> (ø)
...ol-designer/src/components/steplist/IngredPill.tsx 0.00% <0.00%> (ø)
... and 52 more

@sakibh sakibh requested a review from mmencarelli July 6, 2022 14:36
@mmencarelli
Copy link

hey @sakibh is there any way for me to test this in PD? If not, could you send over a video? Thanks!

@sakibh
Copy link
Contributor Author

sakibh commented Jul 6, 2022

hey @sakibh is there any way for me to test this in PD? If not, could you send over a video? Thanks!

Hey @mmencarelli, you can access this branch for PD here: http://sandbox.designer.opentrons.com/pd_add-color-picker/

mmencarelli
mmencarelli requested changes Jul 6, 2022
View reviewed changes
Copy link

@mmencarelli mmencarelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this is amazing!

K, the only change i can see here is that you have the hover state as the enabled state. So move the box shadow over to only appear on hover!

smb2268
smb2268 approved these changes Jul 6, 2022
View reviewed changes
Copy link
Contributor

@smb2268 smb2268 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested this out and it works & looks great!

@mmencarelli
Copy link

Could we have the box-shadow appear when the menu is active/open?

Reference to design on Figma.

Other than that, looks great!

@sakibh sakibh merged commit b4a11af into edge Jul 11, 2022
@sakibh sakibh deleted the pd_add-color-picker branch July 11, 2022 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmencarelli mmencarelli mmencarelli approved these changes

@smb2268 smb2268 smb2268 approved these changes

@b-cooper b-cooper Awaiting requested review from b-cooper

@shlokamin shlokamin Awaiting requested review from shlokamin

Assignees

@sakibh sakibh

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Liquid Setup in PD: add liquid color field Liquid Setup in PD: add custom liquid color picker
3 participants
@sakibh @mmencarelli @smb2268